Skip to content

Make ZeroTreasuryWithdrawals a permanent check in gov state transation rule#5797

Merged
lehins merged 1 commit into
masterfrom
koslambrou/make-zero-withdrawal-permanent-check-master
May 8, 2026
Merged

Make ZeroTreasuryWithdrawals a permanent check in gov state transation rule#5797
lehins merged 1 commit into
masterfrom
koslambrou/make-zero-withdrawal-permanent-check-master

Conversation

@koslambrou

@koslambrou koslambrou commented May 5, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Description

In conwayGovTransition, we remove the condition on hardforkConwayBootstrapPhase for checking ZeroTreasuryWithdrawals. From now on, all eras require that withdrawals (not only post-Conway era) are positive numbers.

We then replayed the preview, preprod and mainnet public chains with this change which guarantees there were no empty treasury withdrawals pre-Conway era.

Here's the proof that we replayed on public networks.

First, here's the same change, but on a ledger version that works with node 10.x: https://github.com/IntersectMBO/cardano-ledger/tree/koslambrou/make-zero-withdrawal-permanent-check. Commit hash is: 357cdd1f47cd3be82a239f4d8bed9ba117effbf8

Then, we used cardano-streamer to replay the chain for each public network.

We first build cardano-streamer with our patch. See the cardano-ledger commit hash which matches the one above:

~/g/i/cardano-streamer ((10.7.1))> cabal build all
remote: Enumerating objects: 1144, done.
remote: Counting objects: 100% (553/553), done.
remote: Compressing objects: 100% (179/179), done.
remote: Total 1144 (delta 223), reused 494 (delta 202), pack-reused 591 (from 3)
Receiving objects: 100% (1144/1144), 825.01 KiB | 2.57 MiB/s, done.
Resolving deltas: 100% (359/359), completed with 23 local objects.
From https://github.com/IntersectMBO/cardano-ledger
   6bae1dbf9..666ce77a6  master                           -> origin/master
 * [new branch]          aniketd/stable-types-for-queries -> origin/aniketd/stable-types-for-queries
 + fdb572d13...fbdf39a87 benchmark-pages                  -> origin/benchmark-pages  (forced update)
 + d70bc5267...59011cd35 f-f/streaming-transitions-2      -> origin/f-f/streaming-transitions-2  (forced update)
 + 7ce2edc34...894c6100d jj/constr-fix                    -> origin/jj/constr-fix  (forced update)
 * [new branch]          lehins/variable-length-tx-fields -> origin/lehins/variable-length-tx-fields
 + b9ee057b8...90aa575ad nm/refactor-ci-testing           -> origin/nm/refactor-ci-testing  (forced update)
 * [new branch]          td/integrate-state-annotated-tx  -> origin/td/integrate-state-annotated-tx
HEAD is now at 357cdd1f4 Make ZeroTreasuryWithdrawals a permanent check in gov state transation rule
Configuration is affected by the following files:
- cabal.project
Up to date

For preview:

~/g/i/cardano-streamer ((10.7.1))> cabal run cardano-streamer:exe:cstreamer -- replay --chain-dir /media/nvme/cardano-node/preview/db --config /media/nvme/cardano-node/preview/config.json --validate full
Configuration is affected by the following files:
- cabal.project
withImmutableDb prepare
withImmutableDb start
Initializing LedgerDb
Done initializing LedgerDb
Starting to Replay
withImmutableDb end
[Conway: EpochNo 755 - SlotNo 65316218] Blocks: 2686680 - Elapsed 02 hours, 30 minutes, 53 seconds

For preprod:

~/g/i/cardano-streamer ((10.7.1))> cabal run cardano-streamer:exe:cstreamer -- replay --chain-dir /media/nvme/cardano-node/preprod/db --config /media/nvme/cardano-node/preprod/config.json --validate full
Configuration is affected by the following files:
- cabal.project
withImmutableDb prepare
withImmutableDb start
Initializing LedgerDb
Done initializing LedgerDb
Starting to Replay
withImmutableDb end
[Conway: EpochNo 286 - SlotNo 122264120] Blocks: 4674240 - Elapsed 03 hours, 50 minutes, 13 seconds

For mainnet:

~/g/i/cardano-streamer ((10.7.1))> cabal run cardano-streamer:exe:cstreamer -- replay --chain-dir /media/nvme/cardano-node/mainnet/db --config /media/nvme/cardano-node/mainnet/config.json --validate full
Configuration is affected by the following files:
- cabal.project
withImmutableDb prepare
withImmutableDb start
Initializing LedgerDb
Done initializing LedgerDb
Starting to Replay
withImmutableDb end
[Conway: EpochNo 629 - SlotNo 186391024] Blocks: 13378340 - Elapsed 001 day, 02 hours, 01 minute, 10 seconds

Fix #5784

Checklist

  • Commits in meaningful sequence and with useful messages.
  • Tests added or updated when needed.
  • CHANGELOG.md files updated for packages with externally visible changes.
    NOTE: New section is never added with the code changes. (See RELEASING.md).
  • Versions updated in .cabal and CHANGELOG.md files when necessary, according to the
    versioning process.
  • Version bounds in .cabal files updated when necessary.
    NOTE: If bounds change in a cabal file, that package itself must have a version increase. (See RELEASING.md).
  • Code formatted (use scripts/fourmolize.sh).
  • Cabal files formatted (use scripts/cabal-format.sh).
  • CDDL files are up to date (use scripts/gen-cddl.sh)
  • hie.yaml updated (use scripts/gen-hie.sh).
  • Self-reviewed the diff.

@koslambrou
koslambrou marked this pull request as ready for review May 5, 2026 13:28
@koslambrou
koslambrou requested a review from a team as a code owner May 5, 2026 13:28
@koslambrou
koslambrou requested a review from lehins May 5, 2026 13:28
@koslambrou
koslambrou force-pushed the koslambrou/make-zero-withdrawal-permanent-check-master branch from 0a53b70 to 1d0e771 Compare May 5, 2026 17:35
lehins
lehins requested changes May 5, 2026
View reviewed changes

@lehins lehins left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unfortunately, the proof was done incorrectly. Because this is a ledger rule validation that we are checking here the command that should have been used is:

$ cabal run cardano-streamer:exe:cstreamer -- replay --chain-dir /media/nvme/cardano-node/preview/db --config /media/nvme/cardano-node/preview/config.json --validate full

Note the --validate full. Or if it was omitted that would have worked too.

The reason why previous test that you did for #5757 had --validate none and it was sufficient, because that was a decoding change, not ledger validation change.

@koslambrou
koslambrou force-pushed the koslambrou/make-zero-withdrawal-permanent-check-master branch from 1d0e771 to 5d5a60c Compare May 7, 2026 14:06
@koslambrou
koslambrou requested a review from lehins May 7, 2026 14:06
@koslambrou

koslambrou commented May 7, 2026

Copy link
Copy Markdown
Contributor Author

@lehins I updated the PR description. I rerun everything with --validate full. The chain replay works.
You'll notice the significant increase in time to completion.

@koslambrou
koslambrou force-pushed the koslambrou/make-zero-withdrawal-permanent-check-master branch 2 times, most recently from 9f85db4 to a21141b Compare May 7, 2026 14:10
@koslambrou @lehins
Make ZeroTreasuryWithdrawals a permanent check in gov state transatio…
c2c95f7 
…n rule

In `conwayGovTransition`, we remove the condition on
`hardforkConwayBootstrapPhase` for checking `ZeroTreasuryWithdrawals`.

We then replayed the preview, preprod and mainnet public chains with
this change which guarentees there were no empty treasury withdrawals
pre-Conway era.
@lehins
lehins force-pushed the koslambrou/make-zero-withdrawal-permanent-check-master branch from a21141b to c2c95f7 Compare May 8, 2026 06:51
lehins
lehins approved these changes May 8, 2026
View reviewed changes

@lehins lehins left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome! Glad we can remove this.
Thank you!

@lehins
lehins enabled auto-merge May 8, 2026 06:54
@lehins
lehins merged commit 81c3ed7 into master May 8, 2026
51 checks passed
@lehins
lehins deleted the koslambrou/make-zero-withdrawal-permanent-check-master branch May 8, 2026 09:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@lehins lehins lehins approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Make ZeroTreasuryWithdrawals a permanent check

2 participants

@koslambrou @lehins